feat: add Vonage multi-channel Messages adapter (#8139)#110
feat: add Vonage multi-channel Messages adapter (#8139)#110deepshekhardas wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
deepshekhardas
force-pushed
the
feat/8139-vonage-messages-adapter
branch
from
ba22934 to
d623ac7
Compare
|
Rebased onto latest main. Could you please review? 🙏 |
Greptile Summary
Confidence Score: 4/5The new Vonage adapters are generally scoped, but the shared constructor path needs attention before merge because normal sends can fail after a successful provider response. Focused coverage exists for the SMS adapter and the review isolated issues in the shared Vonage transport path rather than broad architectural uncertainty. src/Utopia/Messaging/Adapter/VonageTrait.php
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| // This assumes the mock server returns a success response | ||
| // In a real environment, we'd check the request-catcher data | ||
| $this->assertNotEmpty($result); | ||
| } | ||
|
|
||
| public function testSendWhatsApp(): void | ||
| { | ||
| $sender = new VonageWhatsApp($this->applicationId, $this->privateKey); |
There was a problem hiding this comment.
setEndpoint() is undefined on these adapters. None of VonageMessages, VonageWhatsApp, VonageViber, or VonageMMS define a setEndpoint() method — it only exists on Mock. Every test method in this class will crash with "Call to undefined method" before reaching any assertions. The same issue occurs in all four test methods (testSendSMS, testSendWhatsApp, testSendViber, testSendMMS). Either add setEndpoint() / getEndpoint() to VonageTrait, or use an environment variable / constructor parameter to override the base URL.
| public function testSendWhatsApp(): void | ||
| { | ||
| $sender = new VonageWhatsApp($this->applicationId, $this->privateKey); |
There was a problem hiding this comment.
json_decode receives an array, not a string. send() returns array, so \json_decode($response, true) is called with an array argument. In PHP 8 this causes a TypeError; in earlier versions it silently returns null, making assertNotEmpty($result) fail. The same pattern is repeated in all four test methods. Either call send() directly and use assertResponse() (already defined in Base), or cast to JSON before decoding.
deepshekhardas
force-pushed
the
feat/8139-vonage-messages-adapter
branch
from
d623ac7 to
4448c62
Compare
| public function __construct( | ||
| private string $applicationId, | ||
| private string $privateKey, | ||
| private ?string $from = null | ||
| ) { | ||
| } |
There was a problem hiding this comment.
Adapter::__construct() but never calls it, so the base class private $sendCounter property stays uninitialized. A normal new VonageMessages(...)->send($message) reaches Adapter::recordResponse() after process() returns, then reads $this->sendCounter and fails with Typed property Utopia\Messaging\Adapter::$sendCounter must not be accessed before initialization unless callers manually call setTelemetry() first.
Artifacts
Repro: focused PHP harness for VonageMessages send without initialized telemetry
- Contains supporting evidence from the run (text/x-php; charset=utf-8).
Stack trace captured during the T-Rex run
- Keeps the raw stack trace available without making the summary code-heavy.
| headers: [ | ||
| 'Content-Type: application/json', | ||
| 'Authorization: Bearer ' . $jwt, | ||
| 'Accept: application/json', | ||
| ], |
There was a problem hiding this comment.
Adapter::request() expects headers as a list of complete header strings and passes the same array to CURLOPT_HTTPHEADER. This associative array can send malformed headers to cURL, so the Vonage request may miss a valid Authorization or Content-Type header.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Adds a Vonage multi-channel Messages adapter supporting SMS, WhatsApp, Viber, and MMS.
Changes
Testing
Closes #8139